Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

🎉Notion Support #1957

Merged
merged 12 commits into from
Jul 7, 2023
Merged

🎉Notion Support #1957

merged 12 commits into from
Jul 7, 2023

Conversation

jeankhawand
Copy link
Contributor

@jeankhawand jeankhawand commented Jul 1, 2023 •
edited
Loading

➕ miniflux/client/http: patch with json
➕ miniflux/client/http: http request with custom headers

Do you follow the guidelines?

@jeankhawand
🎉 Initial Notion Support
4c1496f 
➕ http patch with json
➕ http custom headers
@jeankhawand
Copy link
Contributor Author

jeankhawand commented Jul 1, 2023 •
edited
Loading

when the user clicks on save - miniflux http client will append new entries as bookmark object on a particular page based on id provided in integrations page. Check sample below:
image

Known Things / Bad experience

  • click on save for more than once will result in duplicate entries (might hit API rate limit)
  • checked previous integration like Wallabag. There is no feedback / alert would mention that an item is saved. Maybe include a migration column called saved of type bool and based on it handle save status ?

@jeankhawand
Copy link
Contributor Author

  • had to add these addition functionalities to http/client PatchJSON method and customHeaders variable to make it work because notion require a custom header Notion-Version to be set
  • In addition a patch method needed to append entries to a page

@jeankhawand jeankhawand mentioned this pull request Jul 1, 2023
Merged
@jeankhawand
Copy link
Contributor Author

updated doc: miniflux/website#58

@jeankhawand jeankhawand mentioned this pull request Jul 1, 2023
Closed
@jeankhawand
🔃 replace http with https in placeholder
4f2ab56 
➕ add notion form labels in French
fguillot
fguillot reviewed Jul 4, 2023
View reviewed changes
Copy link
Member

@fguillot fguillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are few linter issues detected. Regarding the test cases failing due to the translations, see https://miniflux.app/docs/i18n.html#update-translations

database/migrations.go Outdated Show resolved Hide resolved
database/migrations.go Outdated Show resolved Hide resolved
integration/notion/notion.go Show resolved Hide resolved
http/client/client.go Outdated Show resolved Hide resolved
integration/notion/notion.go Outdated Show resolved Hide resolved
@jeankhawand
➕ add missing translations keys for other locales
7af1759 
✍️ get rid of notionURL cause it's not changing for the time being
@jeankhawand jeankhawand changed the title 🎉 Initial Notion Support 🎉Notion Support Jul 4, 2023
@jeankhawand jeankhawand requested a review from fguillot July 4, 2023 09:59
fguillot
fguillot reviewed Jul 6, 2023
View reviewed changes
Copy link
Member

@fguillot fguillot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a linter failure detected: https://github.com/miniflux/v2/actions/runs/5453460586/jobs/9961693138?pr=1957#step:4:25

integration/notion/wrapper.go Show resolved Hide resolved
@jeankhawand jeankhawand requested a review from fguillot July 6, 2023 12:55
@jeankhawand
Copy link
Contributor Author

jeankhawand commented Jul 6, 2023 •
edited
Loading

There is a linter failure detected: https://github.com/miniflux/v2/actions/runs/5453460586/jobs/9961693138?pr=1957#step:4:25

I am not sure why it's failing

fatal: No names found, cannot describe anything.
golint -set_exit_status miniflux.app miniflux.app/api miniflux.app/cli miniflux.app/client miniflux.app/config miniflux.app/crypto miniflux.app/database miniflux.app/errors miniflux.app/fever miniflux.app/googlereader miniflux.app/http/client miniflux.app/http/cookie miniflux.app/http/request miniflux.app/http/response miniflux.app/http/response/html miniflux.app/http/response/json miniflux.app/http/response/xml miniflux.app/http/route miniflux.app/http/server miniflux.app/integration miniflux.app/integration/espial miniflux.app/integration/instapaper miniflux.app/integration/linkding miniflux.app/integration/matrixbot miniflux.app/integration/notion miniflux.app/integration/nunuxkeeper miniflux.app/integration/pinboard miniflux.app/integration/pocket miniflux.app/integration/telegrambot miniflux.app/integration/wallabag miniflux.app/locale miniflux.app/logger miniflux.app/metric miniflux.app/model miniflux.app/oauth2 miniflux.app/proxy miniflux.app/reader/atom miniflux.app/reader/browser miniflux.app/reader/date miniflux.app/reader/encoding miniflux.app/reader/handler miniflux.app/reader/icon miniflux.app/reader/json miniflux.app/reader/media miniflux.app/reader/opml miniflux.app/reader/parser miniflux.app/reader/processor miniflux.app/reader/rdf miniflux.app/reader/readability miniflux.app/reader/rewrite miniflux.app/reader/rss miniflux.app/reader/sanitizer miniflux.app/reader/scraper miniflux.app/reader/subscription miniflux.app/reader/xml miniflux.app/storage miniflux.app/systemd miniflux.app/template miniflux.app/timer miniflux.app/timezone miniflux.app/ui miniflux.app/ui/form miniflux.app/ui/session miniflux.app/ui/static miniflux.app/ui/view miniflux.app/url miniflux.app/validator miniflux.app/version miniflux.app/worker
/workspace/client/client.go:514:1: comment on exported method Client.FetchCounters should be of the form "FetchCounters ..."
/workspace/client/model.go:194:6: exported type FeedCounters should have comment or be unexported
/workspace/client/model.go:242:2: exported const FilterNotStarred should have comment (or a comment on this block) or be unexported
/workspace/config/options.go:51:2: const defaultProxyUrl should be defaultProxyURL
/workspace/config/options.go:127:2: struct field proxyUrl should be proxyURL
/workspace/config/options.go:442:19: method ProxyUrl should be ProxyURL
/workspace/config/options.go:521:1: exported method Options.MetricsUsername should have comment or be unexported
/workspace/config/options.go:525:1: exported method Options.MetricsPassword should have comment or be unexported
/workspace/googlereader/handler.go:551:17: don't use underscores in Go names; var s_err should be sErr
/workspace/http/server/httpd.go:32:1: exported function StartWebServer should have comment or be unexported
/workspace/integration/espial/espial.go:17:2: struct field Url should be URL
/workspace/integration/linkding/linkding.go:16:2: struct field Url should be URL
/workspace/integration/matrixbot/matrixbot.go:15:1: comment on exported function PushEntries should be of the form "PushEntries ..."
/workspace/integration/notion/notion.go:23:1: exported method Client.AddEntry should have comment or be unexported
/workspace/integration/notion/wrapper.go:6:6: exported type Data should have comment or be unexported
/workspace/integration/notion/wrapper.go:10:6: exported type Block should have comment or be unexported
/workspace/integration/notion/wrapper.go:16:6: exported type Bookmark should have comment or be unexported
/workspace/model/categories_sort_options.go:6:1: exported function CategoriesSortingOptions should have comment or be unexported
/workspace/model/enclosure.go:19:20: method Html5MimeType should be HTML5MimeType
/workspace/model/feed.go:42:2: struct field UrlRewriteRules should be URLRewriteRules
/workspace/model/feed.go:61:6: exported type FeedCounters should have comment or be unexported
/workspace/model/feed.go:147:2: struct field UrlRewriteRules should be URLRewriteRules
/workspace/model/feed.go:159:2: struct field UrlRewriteRules should be URLRewriteRules
/workspace/proxy/media_proxy.go:20:6: func name will be used as proxy.ProxyRewriter by other packages, and that stutters; consider calling this Rewriter
/workspace/proxy/proxy.go:23:3: var proxyImageUrl should be proxyImageURL
/workspace/proxy/proxy.go:32:3: var proxyUrl should be proxyURL
/workspace/proxy/proxy.go:46:3: var proxyImageUrl should be proxyImageURL
/workspace/proxy/proxy.go:55:11: if block ends with a return statement, so drop this else and outdent its block
/workspace/proxy/proxy.go:60:3: var proxyUrl should be proxyURL
/workspace/reader/atom/atom_10.go:221:1: receiver name r should be consistent with previous receiver name a for atom10Entry
/workspace/reader/opml/opml.go:19:1: exported function NewOPMLDocument should have comment or be unexported
/workspace/reader/opml/opml.go:19:24: exported func NewOPMLDocument returns unexported type *opml.opmlDocument, which can be annoying to use
/workspace/reader/processor/processor.go:181:6: func getUrlFromEntry should be getURLFromEntry
/workspace/reader/rewrite/rewrite_functions.go:23:2: var youtubeIdRegex should be youtubeIDRegex
/workspace/reader/rewrite/rewrite_functions.go:227:6: func addYoutubeVideoFromId should be addYoutubeVideoFromID
/workspace/reader/rewrite/rewrite_functions.go:318:9: if block ends with a return statement, so drop this else and outdent its block (move short variable declaration to its own line if necessary)
/workspace/reader/rewrite/rewrite_functions.go:356:4: var innerHtml should be innerHTML
/workspace/reader/sanitizer/srcset.go:12:6: exported type ImageCandidate should have comment or be unexported
/workspace/reader/sanitizer/srcset.go:17:6: exported type ImageCandidates should have comment or be unexported
/workspace/reader/sanitizer/truncate.go:8:1: exported function TruncateHTML should have comment or be unexported
/workspace/storage/category.go:236:1: comment on exported method Storage.RemoveAndReplaceCategoriesByName should be of the form "RemoveAndReplaceCategoriesByName ..."
/workspace/storage/enclosure.go:61:1: exported method Storage.GetEnclosure should have comment or be unexported
/workspace/storage/enclosure.go:163:1: exported method Storage.UpdateEnclosure should have comment or be unexported
/workspace/storage/entry.go:366:1: exported method Storage.SetEntriesStatusCount should have comment or be unexported
/workspace/storage/entry.go:390:1: comment on exported method Storage.SetEntriesBookmarkedState should be of the form "SetEntriesBookmarkedState ..."
/workspace/storage/entry_query_builder.go:193:1: exported method EntryQueryBuilder.WithGloballyVisible should have comment or be unexported
/workspace/storage/feed.go:158:1: comment on exported method Storage.FetchCounters should be of the form "FetchCounters ..."
/workspace/ui/unread_entries.go:32:2: var beginSqlCountUnreadEntries should be beginSQLCountUnreadEntries
/workspace/ui/unread_entries.go:42:2: var finishSqlCountUnreadEntries should be finishSQLCountUnreadEntries
/workspace/ui/unread_entries.go:48:2: var beginSqlFetchUnreadEntries should be beginSQLFetchUnreadEntries
/workspace/ui/unread_entries.go:60:2: var finishSqlFetchUnreadEntries should be finishSQLFetchUnreadEntries
/workspace/ui/form/feed.go:22:2: struct field UrlRewriteRules should be URLRewriteRules
/workspace/ui/form/subscription.go:29:2: struct field UrlRewriteRules should be URLRewriteRules
Found 53 lint suggestions; failing.
make: *** [Makefile:109: lint] Error 1

make command throwing this error on different files within the project. In github actions it's mentioning this unnecessary trailing newline (whitespace) it's not indicating which line / file exactly. Could you please guide me through it
Thanks

@fguillot
Copy link
Member

fguillot commented Jul 7, 2023

You can see it in the diff view

https://github.com/miniflux/v2/pull/1957/files#diff-e4a3dbe427c4143ec503ef56321cb20a8302d492f3104c5005e8ccb85200dd94R54

image

@fguillot fguillot merged commit bfb4fc1 into miniflux:main Jul 7, 2023
14 checks passed
@fguillot fguillot linked an issue Aug 13, 2023 that may be closed by this pull request
notion Integrations #1741
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fguillot fguillot Awaiting requested review from fguillot

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

notion Integrations
2 participants
@jeankhawand @fguillot